[Dashboards] Register dashboard locator on server start by nickpeihl · Pull Request #218495 · elastic/kibana

nickpeihl · 2025-04-16T20:58:23Z

Summary

Registers the dashboard locator definition with the share plugin on the Dashboard server start contract.

This moves Dashboard locator code from public to common so that we can expose the Dashboard Locator definition on the server. The getLocation method exposed by the locator does not appear to be used by any server code. So the getDashboardFilterFields function provided to the DashboardLocatorDefinition will throw a not supported error on the server. This should currently never throw since getLocation is not called by the server. But any future implementations where getLocation is called on the server would throw an error and thus would need to augment this function.

For posterity, here's an example where, in the future, we might need to provide a scoped content management client to the DashboardLocatorParams that can be used to load filters from a saved dashboard in the getDashboardFilterFields function.

// in src/platform/plugins/shared/share/server/url_service/http/short_urls/register_create_route.ts

export const registerCreateRoute = (router: IRouter, url: ServerUrlService, contentManagement: ContentManagementServerSetup) => {

router.handleLegacyErrors(async (ctx, req, res) => {
  const savedObjects = (await ctx.core).savedObjects.client;

  const cmClient = contentManagement.contentClient
        .getForRequest({ request: req, requestHandlerContext: ctx })

  const shortUrls = url.shortUrls.get({ savedObjects });
  const { locatorId, params, slug } = req.body;
  const locator = url.locators.get(locatorId);
  // TODO find a way to pass cmClient to the locator. Maybe in params?
  
  try {
  const shortUrl = await shortUrls.create({
    locator,
    params,
    slug,
  });

  return res.ok({
    headers: {
      'content-type': 'application/json',
    },
    body: shortUrl.data,
  });
} catch (error) {

nickpeihl · 2025-04-17T12:06:12Z

+        new DashboardAppLocatorDefinition({
+          useHashedUrl: false,
+          getDashboardFilterFields: async (dashboardId: string) => {
+            return [];


If we want to get the filters from a saved dashboard in the same way we do on the client-side then we'll somehow need to include the RequestHandlerContext so we can scope the content management client to the current user.

Similar to the useHashedUrls example above, we need to use the ctx argument to create a content management client scoped to the user to pass to the locator.

// in src/platform/plugins/shared/share/server/url_service/http/short_urls/register_create_route.ts export const registerCreateRoute = (router: IRouter, url: ServerUrlService, contentManagement: ContentManagementServerSetup) => { ... router.handleLegacyErrors(async (ctx, req, res) => { const savedObjects = (await ctx.core).savedObjects.client; const cmClient = contentManagement.contentClient .getForRequest({ request: req, requestHandlerContext: ctx }) const shortUrls = url.shortUrls.get({ savedObjects }); const { locatorId, params, slug } = req.body; const locator = url.locators.get(locatorId); // TODO find a way to pass cmClient to the locator. Maybe in params? try { const shortUrl = await shortUrls.create({ locator, params, slug, }); return res.ok({ headers: { 'content-type': 'application/json', }, body: shortUrl.data, }); } catch (error) {

AFAICT, the getLocation method from the LocatorPublic interface is never actually called by the server. So, it currently does not matter on the Dashboard server contract what we set for useHashedUrls or the return value of getDashboardFilterFields because, currently, they are never be used by Kibana server. It seems that the getLocation is only currently ever used by the client.

Also, the URLService on the Share server plugin only exposes certain locator methods but they are not supported on the server. So I suspect we can safely assume the getLocation is not directly exposed on the server side URL service and it will likely not get called from the server. Maybe someone from @elastic/appex-sharedux can keep me honest here?

So I think we have two options for registering the Dashboard locator on the server:

Have getDashboardFilterFields always return an empty array. Currently, this function is never called. But any future implementations where getLocation is called on the server would not retrieve any saved filters on a dashboard. So it would not break, but might return unexpected results.

Have getDashboardFilterFields throw an error as not supported on the server. Again, this should currently never throw since getLocation is not called by the server. But any future implementations where getLocation is called on the server would throw an error. cc @ThomThomson

Good find! That will definitely make this simpler. I'd suggest making it throw an error on the serverside - which will make sure we revisit this if we ever need to call getLocation on the server.

nickpeihl · 2025-04-17T12:10:36Z

+    if (plugins.share) {
+      plugins.share.url.locators.create(
+        new DashboardAppLocatorDefinition({
+          useHashedUrl: false,


If we want to support hashed urls on the server the same way we do on the client-side, we'll need the RequestHandlerContext) so we can scope the server-side UISettingsServiceStart.asScopedToClient to retrieve the 'state:storeInSessionStorage' setting as we do in the client.

For example: The short URL create route passes the ctx: RequestHandlerContext. We use this to scope the savedObjects client to the user making the request. To retrieve the 'state:storeInSessionStorage' setting we need to do something like this:

// in src/platform/plugins/shared/share/server/url_service/http/short_urls/register_create_route.ts router.handleLegacyErrors(async (ctx, req, res) => { const savedObjects = (await ctx.core).savedObjects.client; const uiSettingsClient = core.uiSettings.asScopedToClient(soClient); const useHashedUrl = uiSettingsClient.get('state:storeInSessionStorage'); const shortUrls = url.shortUrls.get({ savedObjects }); const { locatorId, params, slug } = req.body; const locator = url.locators.get(locatorId); // TODO find a way to pass useHashedUrl to the locator. Maybe in params? try { const shortUrl = await shortUrls.create({ locator, params, slug, }); return res.ok({ headers: { 'content-type': 'application/json', }, body: shortUrl.data, }); } catch (error) {

FWIW, the server-side Discover locator does not support hashed URLs.

I'd say it's safe to not support hashing on the serverside locator for Dashboards.

…locator-server

…s` param.

elasticmachine · 2025-04-18T11:54:43Z

Pinging @elastic/kibana-presentation (Team:Presentation)

…ctory

nickpeihl · 2025-04-18T18:48:59Z

-
-export const UI_SETTINGS = {
-  ENABLE_LABS_UI: 'labs:dashboard:enable_ui',
-};


These top level exports were removed to avoid bloating the bundle size. Aside from types, the imports from common in dashboard/public have been changed to deep imports.

rmyz

x-pack/solutions/observability/plugins/inventory/public/hooks/use_detail_view_redirect.ts changes LGTM

jgowdyelastic

ML changes LGTM

ThomThomson

Changes LGTM! Mostly import changes, code only review.

ThomThomson · 2025-04-24T14:51:27Z

+        new DashboardAppLocatorDefinition({
+          useHashedUrl: false,
+          getDashboardFilterFields: async (dashboardId: string) => {
+            throw new Error(


Thanks for throwing an error here!

Ikuni17 · 2025-04-24T19:18:10Z

Removed Operations as we no longer have code owner changes in this PR.

kibanamachine · 2025-04-28T15:23:35Z

Starting backport for target branches: 8.19

https://github.com/elastic/kibana/actions/runs/14711519816

elasticmachine · 2025-04-28T15:24:28Z

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`dashboard`	92	82	-10

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`dashboard`	548.0KB	548.1KB	+22.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`dashboard`	11	10	-1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`dashboard`	17.7KB	17.8KB	+132.0B

Unknown metric groups

API count

id	before	after	diff
`dashboard`	96	88	-8

History

💛 Build #295580 was flaky 9d11723
💚 Build #294579 succeeded 10aa7dc
💔 Build #294448 failed dc1cf22
💛 Build #294400 was flaky be4a587
💛 Build #294332 was flaky 25f3446
💔 Build #294317 failed 48b0df0

kibanamachine · 2025-04-28T15:29:33Z

💔 All backports failed

Status	Branch	Result
❌	8.19	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 218495

Questions ?

Please refer to the Backport tool documentation

nickpeihl · 2025-04-30T12:30:28Z

💚 All backports created successfully

Status	Branch	Result
✅	8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

## Summary Registers the dashboard locator definition with the share plugin on the Dashboard server start contract. This moves Dashboard locator code from public to common so that we can expose the Dashboard Locator definition on the server. The `getLocation` method exposed by the locator does not appear to be used by any server code. So the `getDashboardFilterFields` function provided to the `DashboardLocatorDefinition` will throw a not supported error on the server. This should currently never throw since getLocation is not called by the server. But any future implementations where getLocation is called on the server would throw an error and thus would need to augment this function. For posterity, here's an example where, in the future, we might need to provide a scoped content management client to the DashboardLocatorParams that can be used to load filters from a saved dashboard in the `getDashboardFilterFields` function. ```ts // in src/platform/plugins/shared/share/server/url_service/http/short_urls/register_create_route.ts export const registerCreateRoute = (router: IRouter, url: ServerUrlService, contentManagement: ContentManagementServerSetup) => { router.handleLegacyErrors(async (ctx, req, res) => { const savedObjects = (await ctx.core).savedObjects.client; const cmClient = contentManagement.contentClient .getForRequest({ request: req, requestHandlerContext: ctx }) const shortUrls = url.shortUrls.get({ savedObjects }); const { locatorId, params, slug } = req.body; const locator = url.locators.get(locatorId); // TODO find a way to pass cmClient to the locator. Maybe in params? try { const shortUrl = await shortUrls.create({ locator, params, slug, }); return res.ok({ headers: { 'content-type': 'application/json', }, body: shortUrl.data, }); } catch (error) { ``` (cherry picked from commit 8e1a426) # Conflicts: # src/platform/plugins/shared/dashboard/public/dashboard_top_nav/dashboard_favorite_button.tsx # x-pack/platform/plugins/shared/ml/public/application/jobs/new_job/job_from_dashboard/quick_create_job_base.ts

…) (#219717) # Backport This will backport the following commits from `main` to `8.19`: - [[Dashboards] Register dashboard locator on server start (#218495)](#218495)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)

## Summary Registers the dashboard locator definition with the share plugin on the Dashboard server start contract. This moves Dashboard locator code from public to common so that we can expose the Dashboard Locator definition on the server. The `getLocation` method exposed by the locator does not appear to be used by any server code. So the `getDashboardFilterFields` function provided to the `DashboardLocatorDefinition` will throw a not supported error on the server. This should currently never throw since getLocation is not called by the server. But any future implementations where getLocation is called on the server would throw an error and thus would need to augment this function. For posterity, here's an example where, in the future, we might need to provide a scoped content management client to the DashboardLocatorParams that can be used to load filters from a saved dashboard in the `getDashboardFilterFields` function. ```ts // in src/platform/plugins/shared/share/server/url_service/http/short_urls/register_create_route.ts export const registerCreateRoute = (router: IRouter, url: ServerUrlService, contentManagement: ContentManagementServerSetup) => { router.handleLegacyErrors(async (ctx, req, res) => { const savedObjects = (await ctx.core).savedObjects.client; const cmClient = contentManagement.contentClient .getForRequest({ request: req, requestHandlerContext: ctx }) const shortUrls = url.shortUrls.get({ savedObjects }); const { locatorId, params, slug } = req.body; const locator = url.locators.get(locatorId); // TODO find a way to pass cmClient to the locator. Maybe in params? try { const shortUrl = await shortUrls.create({ locator, params, slug, }); return res.ok({ headers: { 'content-type': 'application/json', }, body: shortUrl.data, }); } catch (error) { ```

nickpeihl added 2 commits April 16, 2025 15:33

Move dashboard locator to common

9a17618

Register dashboard locator on server start

e588d3c

kowalczyk-krzysztof mentioned this pull request Apr 17, 2025

[Sharing URLs] Add the ability to switch between relative and absolute time range #218056

Merged

nickpeihl commented Apr 17, 2025

View reviewed changes

nickpeihl added 4 commits April 17, 2025 11:56

Fix imports

17b3d61

Merge remote-tracking branch 'upstream/main' into register-dashboard-…

3e34a6d

…locator-server

Add common to extraPublicDirs

48b0df0

Throw an error if getLocation is called with the `preserveSavedFilter…

6a9ed87

…s` param.

nickpeihl added backport:version Backport to applied version labels v9.1.0 v8.19.0 Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// release_note:skip Skip the PR/issue when compiling release notes labels Apr 17, 2025

nickpeihl added 2 commits April 17, 2025 15:49

Fix imports

b7f1f4e

Update limits 🙈

25f3446

nickpeihl marked this pull request as ready for review April 18, 2025 11:54

nickpeihl requested a review from a team as a code owner April 18, 2025 11:54

nickpeihl requested review from a team April 18, 2025 11:54

nickpeihl requested review from a team as code owners April 18, 2025 11:54

nickpeihl marked this pull request as draft April 18, 2025 12:45

nickpeihl added 3 commits April 18, 2025 09:18

Re-export necessary parts of common instead of the entire common dire…

be4a587

…ctory

Use deep exports from common to avoid bundle bloat

8fa992d

Revert limits change

dc1cf22

nickpeihl commented Apr 18, 2025

View reviewed changes

nickpeihl added 2 commits April 21, 2025 08:23

Fix type error on test

e8f1279

Merge branch 'main' into register-dashboard-locator-server

10aa7dc

nickpeihl marked this pull request as ready for review April 21, 2025 16:40

jloleysens removed the request for review from a team April 22, 2025 08:54

rmyz approved these changes Apr 22, 2025

View reviewed changes

botelastic Bot added the Feature:Drilldowns Embeddable panel Drilldowns label Apr 22, 2025

Merge branch 'main' into register-dashboard-locator-server

9d11723

jgowdyelastic approved these changes Apr 24, 2025

View reviewed changes

ThomThomson approved these changes Apr 24, 2025

View reviewed changes

Ikuni17 removed the request for review from a team April 24, 2025 19:17

semd approved these changes Apr 25, 2025

View reviewed changes

flash1293 approved these changes Apr 28, 2025

View reviewed changes

Merge branch 'main' into register-dashboard-locator-server

5d20701

nickpeihl enabled auto-merge (squash) April 28, 2025 13:56

nickpeihl merged commit 8e1a426 into elastic:main Apr 28, 2025
11 checks passed

nickpeihl mentioned this pull request Apr 30, 2025

[8.19] [Dashboards] Register dashboard locator on server start (#218495) #219717

Merged

Conversation

nickpeihl commented Apr 16, 2025 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

nickpeihl Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickpeihl Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomThomson Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

nickpeihl Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomThomson Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Apr 18, 2025

Uh oh!

nickpeihl Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rmyz left a comment

Choose a reason for hiding this comment

Uh oh!

jgowdyelastic left a comment

Choose a reason for hiding this comment

Uh oh!

ThomThomson left a comment

Choose a reason for hiding this comment

Uh oh!

ThomThomson Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

Ikuni17 commented Apr 24, 2025

Uh oh!

Uh oh!

kibanamachine commented Apr 28, 2025

Uh oh!

elasticmachine commented Apr 28, 2025

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

History

Uh oh!

kibanamachine commented Apr 28, 2025

💔 All backports failed

Manual backport

Questions ?

Uh oh!

nickpeihl commented Apr 30, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

nickpeihl commented Apr 16, 2025 •

edited by kibanamachine

Loading

nickpeihl Apr 17, 2025 •

edited

Loading

nickpeihl Apr 17, 2025 •

edited

Loading

nickpeihl Apr 17, 2025 •

edited

Loading

nickpeihl Apr 18, 2025 •

edited

Loading